Skip to content

WIP: Add search functionality#10

Open
jardelima wants to merge 4 commits into
Nick-Gabe:mainfrom
jardelima:feature/add_search
Open

WIP: Add search functionality#10
jardelima wants to merge 4 commits into
Nick-Gabe:mainfrom
jardelima:feature/add_search

Conversation

@jardelima

Copy link
Copy Markdown
Contributor

Add search functionality.

@vercel

vercel Bot commented Mar 10, 2025

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
dev-tok ❌ Failed (Inspect) Mar 10, 2025 6:38pm

@jardelima

Copy link
Copy Markdown
Contributor Author

I need of the a light about GET or logic of the search, please. 🙏

@Nick-Gabe Nick-Gabe left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I actually couldn't see the appearance since the preview deployment failed.
So I only analyzed the code, but overall it's going in a nice direction! :)

Also sorry for reviewing it soo late, I didn't see this pr 😅

const { t } = useTranslation();

const [isOpen, setIsOpen] = useState(false);
const [historic, setHistoric] = useState<Array<string>>([]);

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pls also change other types to [] and historic to history

Suggested change
const [historic, setHistoric] = useState<Array<string>>([]);
const [history, setHistory] = useState<string[]>([]);

}

export const SearchComponent = forwardRef<SearchComponentHandler>((_, ref) => {
const { t } = useTranslation();

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
const { t } = useTranslation();
const { t } = useTranslation();

const backdropRef = useRef<HTMLDivElement>(null);
const navigate = useNavigate();

const storedItems = useRef<Array<string>>(JSON.parse(localStorage.getItem("searchItems") || "[]")).current;

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think "previousSearches" is a better name, both for variable and storage

const queryParams = new URLSearchParams(location.search);
const query = queryParams.get("query");

// ADD SEARCH LOGIC

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe it would use Dev.to's algolia, for example here's the full fetch it does for searching "javascript":

fetch("https://prsobfp46h-dsn.algolia.net/1/indexes/Article_production/query?x-algolia-agent=Algolia%20for%20JavaScript%20(4.23.3)%3B%20Browser%20(lite)&x-algolia-api-key=9aa7d31610cba78851c9b1f63776a9dd&x-algolia-application-id=PRSOBFP46H", {
  "headers": {
    "accept": "*/*",
    "accept-language": "en-US,en;q=0.9,pt;q=0.8",
    "cache-control": "no-cache",
    "content-type": "application/x-www-form-urlencoded",
    "pragma": "no-cache",
    "priority": "u=1, i",
    "sec-ch-ua": "\"Chromium\";v=\"137\", \"Not/A)Brand\";v=\"24\"",
    "sec-ch-ua-mobile": "?0",
    "sec-ch-ua-platform": "\"macOS\"",
    "sec-fetch-dest": "empty",
    "sec-fetch-mode": "cors",
    "sec-fetch-site": "cross-site"
  },
  "referrer": "https://dev.to/",
  "referrerPolicy": "strict-origin-when-cross-origin",
  "body": "{\"query\":\"javascript\",\"hitsPerPage\":\"60\",\"queryType\":\"prefixNone\",\"page\":\"0\"}",
  "method": "POST",
  "mode": "cors",
  "credentials": "omit"
});

but it seems indeed a bit complicated, so if you prefer i can continue this myself, just let me know! 🫂

Comment thread package.json
"react-dom": "^19.0.0",
"react-i18next": "^15.4.1",
"react-infinite-scroll-component": "^6.1.0",
"react-router-dom": "^7.3.0",

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

react-router-dom is actually kind of deprecated, they recommend to use react-router now

toggle: () => void;
}

export const SearchComponent = forwardRef<SearchComponentHandler>((_, ref) => {

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Technically, instead of using a ref you could simply use an external state which decides to render or not this component. But I liked your idea actually, it adds a bit more complexity but goes in the same way as TikTok, which for example shows "search buttons" in comments, that when clicked open the search from there and perform it straight away.

Good job! 😄👍

Comment on lines +23 to +25
toggle: () => {
setIsOpen(!isOpen);
}

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of toggle, what do you think of two separate functions: show/hide
whereas show also receives an argument specifying the search to be done.

>
<div className="p-6">
<button
onClick={() => setIsOpen(!isOpen)}

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like to think of this conceptually. Would it make sense for a close button to end up opening the search? Given this, it's better to be explicit

Suggested change
onClick={() => setIsOpen(!isOpen)}
onClick={() => setIsOpen(false)}

id="search"
placeholder={t("search.searchPlaceholder")}
ref={searchRef}
className="bg-white w-full h-10 px-4 outline-none text-black placeholder:text-black"

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is placeholder color needed? normal text-black does not affect it?

localStorage.setItem("searchItems", JSON.stringify(newHistoric));
}

const removeAllSearchHistoricItem = () => {

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
const removeAllSearchHistoricItem = () => {
const removeAllPreviousSearches = () => {

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants